-
-
Notifications
You must be signed in to change notification settings - Fork 51
Integration test password grant #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@jorenvandeweyer I think this is a very good approach. What I'd like to see, since this is a good chance at least, is to stronger bind these tests to paragraphs in the standard. For example we could use the phrasing from the standard(s) when writing the (un)its or refer to the sections, if possible. |
@jorenvandeweyer I have a question - do you propose these tests, because the current password grant tests are incomplete or do you want to test more towards standard compliance? If second - I would propose to refactor the paths from Then another thing on the |
The current "integration" tests are currently testing the grant flows step by step. I think the real integration tests that test the complete flow from start to end are missing. It think it's an opportunity to test the compliance in these tests too. I'll finish this pull request this weekend, and include a factory for the db. |
I read quickly over it. It seems about right. Also it does not touch any old code, so there is no risk, that we break something. I personally dont like the should notation, but hey... the other unit tests are also in should notation, so for having the same principle over the whole project, I will not demand a change. :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two minor things but overall a very welcomed addition
I think I implemented all feedback. Can this be merged? I want to write some compliance tests for the other grant types in a different pull request. |
I'm trying to write some actual integration tests for the library and want some feedback before I cover all the other grants.
Summary
Password grant integration test.
Linked issue(s)
#42
Involved parts of the project
Added tests?